Implement the changes necessary to use the new custom editor #12188

rchiodo · 2020-06-08T16:31:27Z

For #10744

This changes our custom editor support to work with the final API that VS code shipped in 1.45.

Note it works a lot better in the insider's version though. Seems they've fixed some bugs with it.

…tom_editor

fix delete/insert commands to undo properly

…tom_editor

…xperiment

rchiodo · 2020-06-08T16:36:37Z

Ah actually hold on. I don't think the experiment flag is working. I think the section in the package.json forces custom editor usage regardless of if I register the custom editor or not.

rchiodo · 2020-06-08T17:24:42Z

Okay sorry about that. Should be dynamically updating package.json now

DonJayamanne · 2020-06-08T19:05:03Z

data/.vscode/settings.json

@@ -1,3 +1,3 @@
 {
-    "python.pythonPath": "/usr/bin/python3"
+    "python.pythonPath": "C:\\Users\\rchiodo.REDMOND\\AppData\\Local\\Continuum\\miniconda3\\envs\\jupyter\\python.exe"


Might want to revert

DonJayamanne · 2020-06-08T19:05:10Z

.vscode/launch.json

@@ -25,7 +25,7 @@
                // Enable this to turn on redux logging during debugging
                "XVSC_PYTHON_FORCE_LOGGING": "1",
                // Enable this to try out new experiments locally
-                "XVSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE": "1",
+                "VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE": "1",


Actually this was on purpose. Otherwise experiments won't load from the json file.

src/client/common/application/customEditorService.ts

DonJayamanne · 2020-06-08T19:13:42Z

src/client/datascience/interactive-ipynb/notebookStorageProvider.ts

-        const fileUri = Uri.file(fileName);
-        // Turn this back into an untitled
-        return fileUri.with({ scheme: 'untitled', path: fileName });
+        return Uri.parse(`untitled:///${os.tmpdir}/${fileName}`);


Please make this conditional.
I.e. do this only if in custom editor experiment. Else old behavior.
Else existing code and VS Code Notebooks will end up with temp files which we don't need. I.e. we need this approach only for custom editor.

Sorry it doesn't work anymore once we switch to VS code 1.45. Their URI parser requires a path.

However the behavior under the covers is the same.

Meaning for the webview, having 'untitled' on the schema is enough. It doesn't matter what the path is.

In reply to: 436957206 [](ancestors = 436957206)

But the URI.parse function requires a path now.

In reply to: 436965139 [](ancestors = 436965139,436957206)

DonJayamanne · 2020-06-08T19:17:45Z

src/client/datascience/types.ts

@@ -280,7 +280,7 @@ export interface IGatherLogger extends INotebookExecutionLogger {
 export const IJupyterExecution = Symbol('IJupyterExecution');
 export interface IJupyterExecution extends IAsyncDisposable {
    sessionChanged: Event<void>;
-    serverStarted: Event<INotebookServerOptions>;
+    serverStarted: Event<INotebookServerOptions | undefined>;


Not sure why this is nullable now?

Moving to VS code 1.45 requires all events to pass a value. It's no longer optional

DonJayamanne · 2020-06-08T19:18:27Z

src/client/datascience/jupyterDebugService.ts

@@ -78,7 +78,7 @@ export class JupyterDebugService implements IJupyterDebugService, IDisposable {
    private breakpointEmitter: EventEmitter<void> = new EventEmitter<void>();
    private debugAdapterTrackerFactories: DebugAdapterTrackerFactory[] = [];
    private debugAdapterTrackers: DebugAdapterTracker[] = [];
-    private sessionChangedEvent: EventEmitter<DebugSession> = new EventEmitter<DebugSession>();
+    private sessionChangedEvent: EventEmitter<DebugSession | undefined> = new EventEmitter<DebugSession>();


Why is this also nullable now? Is this a related change?

See other comment about events.

DonJayamanne · 2020-06-08T19:19:26Z

src/datascience-ui/native-editor/redux/reducers/creation.ts

@@ -114,6 +114,34 @@ export namespace Creation {
        return result;
    }

+    export function insertExistingAbove(arg: NativeEditorReducerArg<ICellAction & { cell: ICell }>): IMainState {


Is this related to custom editor or existing code as well?
Don't see why this needs to change

This was a bug in delete undo.

rchiodo · 2020-06-08T19:50:32Z

src/datascience-ui/native-editor/redux/reducers/creation.ts

@@ -114,6 +114,34 @@ export namespace Creation {
        return result;
    }

+    export function insertExistingAbove(arg: NativeEditorReducerArg<ICellAction & { cell: ICell }>): IMainState {
+        const newVM = prepareCellVM(arg.payload.data.cell, false, arg.prevState.settings);


arg.payload.data.cell [](start = 36, length = 21)

THis line right here is the difference. Undoing a delete woudl put in a blank cell. This change here puts in the cell that came with the message.

src/client/common/application/customEditorService.ts

src/client/common/experiments/groups.ts

sonarqubecloud · 2020-06-08T20:40:44Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
6 Code Smells

No Coverage information
0.0% Duplication

* Turn on custom editor again * Fix copy problem * Fix copy problem again * Get undo/redo to work * Some fixes for synching more than one editor * Fix untitled. Fix timeouts * fix command manager to not be used. Not necessary with temp path. fix delete/insert commands to undo properly * Fix functional test * Add experiment * Since package json is enabling proposed api, turn of notebooks with experiment * Turn off proposed api * Upgrade VS code to 1.45 so can use new api outside of insiders * Update package.json dynamically * Fix merge code to handle arrays * Fix unit tests. Backup happens at the provider level now * Fix hygiene * Fix untitled to work for old provider too * Some feedback and attempt to fix URI problem on linux/mac * Review feedback

* Implement the changes necessary to use the new custom editor (#12188) * Turn on custom editor again * Fix copy problem * Fix copy problem again * Get undo/redo to work * Some fixes for synching more than one editor * Fix untitled. Fix timeouts * fix command manager to not be used. Not necessary with temp path. fix delete/insert commands to undo properly * Fix functional test * Add experiment * Since package json is enabling proposed api, turn of notebooks with experiment * Turn off proposed api * Upgrade VS code to 1.45 so can use new api outside of insiders * Update package.json dynamically * Fix merge code to handle arrays * Fix unit tests. Backup happens at the provider level now * Fix hygiene * Fix untitled to work for old provider too * Some feedback and attempt to fix URI problem on linux/mac * Review feedback * Update changelog * Remove news entry

* Port custom editor changes to release branch (#12222) * Implement the changes necessary to use the new custom editor (#12188) * Turn on custom editor again * Fix copy problem * Fix copy problem again * Get undo/redo to work * Some fixes for synching more than one editor * Fix untitled. Fix timeouts * fix command manager to not be used. Not necessary with temp path. fix delete/insert commands to undo properly * Fix functional test * Add experiment * Since package json is enabling proposed api, turn of notebooks with experiment * Turn off proposed api * Upgrade VS code to 1.45 so can use new api outside of insiders * Update package.json dynamically * Fix merge code to handle arrays * Fix unit tests. Backup happens at the provider level now * Fix hygiene * Fix untitled to work for old provider too * Some feedback and attempt to fix URI problem on linux/mac * Review feedback * Update changelog * Remove news entry * Port run by line stepping fix to release (#12223) * fix continue event to run cell (#12211) Co-authored-by: Ian Huff <ianhuff@alex-x230.northamerica.corp.microsoft.com> * update changelog and delete news file Co-authored-by: Ian Huff <ianhuff@alex-x230.northamerica.corp.microsoft.com> * Port run by line stop becoming interrupt to the release branch (#12271) * Run by line changes for release (#12256) * Run by line changes for release * Translate step tooltip and make sure F10 works outside of already debugging * Update changelog * Cherry-pick fixes from master to june release (#12282) * Merge stuff from may release back into master (#12233) * Revert vscode-extension-telemetry changes for the release (#11602) (#11656) * Revert "Fix slashes in telemetry unit tests (#11572)" This reverts commit 7431c9c. * Revert "Use vscode-extension-telemetry for our exceptions & error telemetry (#11524)" This reverts commit d5065e6. * Remove from changelog * Port storage fix to release branch (#11673) * Fix storage not being used (#11649) * Fix storage not being used * Add disposable to storage so it won't write after shutdown * Fix dirty title * Hack to get tests to pass * Another way to get run all to not interfere * Update changelog * Port scrolling fix to release (#11688) * Fix scrolling (#11681) * Fix scrolling * Review feedback - fix scrolling on expand/collapse * Update changelog * Update package.json Co-authored-by: Jim Griesmer <jimgries@microsoft.com> * Cherry-pick pipenv changes and pythonpath prompt changes to release (#11700) * Show the prompt again if user clicks on more info (#11664) * Show the prompt again if user clicks on more info * Review feedback * Use Learn more as text for the link. * Leave pipenv in a corner until the user decides to select an interpreter (#11654) * add onSuggestion option * Swap onActivation with onSuggestion * Update unit tests * Remove registration of IPipenvService * Move didTriggerInterpreterSuggestions logic inside pipenv locator * Fix existing unit tests * Add new unit tests * Replace typemoq any param with object * Shorten the tests * Fix warning * Duplicate teardown Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> * Update extension version (#11730) * Update extension version * Update date in changelog. * Update change log with additional notes. (#11764) * Cherry picks and version updates for bug fix release (#11878) * Do not execute shebang as an interpreter until user has clicked on the codelens enclosing the shebang (#11816) * Do not execute shebang as an interpreter until user has clicked on the codelens enclosing the shebang * Rename * Oops * Update src/test/providers/shebangCodeLenseProvider.unit.test.ts Co-authored-by: Karthik Nadig <kanadig@microsoft.com> Co-authored-by: Karthik Nadig <kanadig@microsoft.com> * Update version and change log for bugfix release Co-authored-by: Kartik Raj <karraj@microsoft.com> * Cherry-pick ExP platform work from master (#12160) * User cannot belong to all experiments in an experiment group (#11945) * 10790 prep - Create an experiments/ folder (#11980) * experiments.ts -> experiments/manager.ts * eexperimentGroups -> experiments/experimentGroups * test/experiments.u.t -> test/experiments/manager.u.t * experimentGroups -> groups * Whoops committed one file too many * Add support for VS Code's experiment service (#11979) * Add npm package * News entry * experiments.ts -> experiments/manager.ts * Wrong issue number * eexperimentGroups -> experiments/experimentGroups * Move experiments.unit.tests.ts -> experiments/ * Commit new files * Merge gone sideways * Add types * Add opt in/out handling * Activation (service registry) * Don't pin tas-client to one version * Unit tests + fixes * Lol forgot to remove a comment + add headers * Forgot 'use strict' in service.ts * Use IApplicationEnvironment instead of IExtensions * Apply suggestions from code review Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> * Remove unnecessary formatted props * n e v e r m i n d * Aight fixed it * flight -> experiment * Check stub calls instead of ctor impl * removed getExperimentService stub check * Set shared properties for all telemetry events * Add test for shared properties Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> * Fix merge issues * Fix index unit tests * Revert "Fix index unit tests" This reverts commit 2fb61fc. * Make MPLS and vscode-extension-telemetry work together 🤝 (#11823) * Revert "Revert vscode-extension-telemetry changes for the release (#11602)" This reverts commit 71d1793. * Remove entry from changelog + add new news entry * Update LS code to use periods instead of slashes * Revert "Update LS code to use periods instead of slashes" This reverts commit 1356651. * Replace slashes before sending telemetry instead * Too fast too furious * Fix more merge issues Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> * Update version and changelog for point release (#12171) * Ensure extension features are started when in Deprecate PythonPath experiment and opening a file without any folder opened (#12182) * Ensure extension features are started when in Deprecate PythonPath experiment and opening a file without any folder opened. * Added comments * Update change log (#12190) * Cherry pick fix for hasInterpreters and update change log (#12198) * Double-check for interpreters when running diagnostics (#12158) * Get interpreters if hasInterpreters returns false * Undo ignoreErrors() * Add unit tests * Fixed tests * Newline * Fix merge issues. * Update change log Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> * Update version for point release (#12259) Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Co-authored-by: Rich Chiodo <rchiodo@users.noreply.github.com> Co-authored-by: Jim Griesmer <jimgries@microsoft.com> Co-authored-by: Kartik Raj <karraj@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> * Make Jedi the default LS (#12226) * Make Jedi the default LS * Add news entry * Update change log Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Co-authored-by: Rich Chiodo <rchiodo@users.noreply.github.com> Co-authored-by: Jim Griesmer <jimgries@microsoft.com> Co-authored-by: Kartik Raj <karraj@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> Co-authored-by: Luciana Abud <45497113+luabud@users.noreply.github.com> * Port raw auto start fix to release (#12288) * don't return a connection from the raw provider for getOnly (#12277) Co-authored-by: Ian Huff <ianhuff@alex-x230.northamerica.corp.microsoft.com> * update changelog and remove news entry Co-authored-by: Ian Huff <ianhuff@alex-x230.northamerica.corp.microsoft.com> * Rchiodo/rel june update (#12356) * Update security problems * Update release version * Update 3rd party notices * Add font awesome back to third party notices * ExP telemetry fixes (#12358) * Use extension channel instead of VS Code channel * Use publisher.name extension id (not just name) * Update vscode-tas-client Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> * Port start page to release (#12365) * start page experiment and fixes (#12339) * move EnableStartPage enum to groups.ts * - remove the command from the experiment - fix the start page not loading when you go back to a priviews version and then updating again - update some labels in the start page - update the sample notebook to include run by line - the sample notebook now has a title - updated the images from the sample notebook * oops * oops * -add messages to open folder and workspace -add telemetry to measure if the webview error happens -give space between bullet points -add link to enable experiments to the sample notebook -get the release notes from a different file -fix bullet points in the sample notebook -sample notebook now opens with a counter in the title * fix a unit test * final changes * final final changes * final^3 changes * removed run by line from the sample notebook * Fixup after merge Co-authored-by: David Kutugata <dakutuga@microsoft.com> * Fixup bad merge * Last changes before release (#12371) Co-authored-by: Ian Huff <ianhu@microsoft.com> Co-authored-by: Ian Huff <ianhuff@alex-x230.northamerica.corp.microsoft.com> Co-authored-by: Karthik Nadig <kanadig@microsoft.com> Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Co-authored-by: Jim Griesmer <jimgries@microsoft.com> Co-authored-by: Kartik Raj <karraj@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> Co-authored-by: Luciana Abud <45497113+luabud@users.noreply.github.com> Co-authored-by: David Kutugata <dakutuga@microsoft.com>

rchiodo added 17 commits May 27, 2020 15:46

Turn on custom editor again

bb5e0ad

Fix copy problem

66ea1dd

Fix copy problem again

d27e2f6

Merge remote-tracking branch 'origin/master' into rchiodo/renable_cus…

08c0760

…tom_editor

Get undo/redo to work

e49e9ad

Merge remote-tracking branch 'origin/master' into rchiodo/renable_cus…

d167a10

…tom_editor

Some fixes for synching more than one editor

0f48e1e

Merge remote-tracking branch 'origin/master' into rchiodo/renable_cus…

d92a98b

…tom_editor

Fix untitled. Fix timeouts

cc921af

fix command manager to not be used. Not necessary with temp path.

c3a470c

fix delete/insert commands to undo properly

Fix functional test

ed58c18

Merge remote-tracking branch 'origin/master' into rchiodo/renable_cus…

80b881f

…tom_editor

Add experiment

d101156

Merge remote-tracking branch 'origin/master' into rchiodo/renable_cus…

6b3d3b6

…tom_editor

Since package json is enabling proposed api, turn of notebooks with e…

2922e7c

…xperiment

Turn off proposed api

384e38c

Upgrade VS code to 1.45 so can use new api outside of insiders

a6a9479

rchiodo requested review from IanMatthewHuff, DonJayamanne, greazer, DavidKutu, joyceerhl and techwithtim June 8, 2020 16:31

rchiodo self-assigned this Jun 8, 2020

rchiodo marked this pull request as draft June 8, 2020 16:36

rchiodo added 2 commits June 8, 2020 10:01

Update package.json dynamically

a115152

Fix merge code to handle arrays

7861c7d

rchiodo marked this pull request as ready for review June 8, 2020 17:24

rchiodo added 3 commits June 8, 2020 10:42

Fix unit tests. Backup happens at the provider level now

4715a36

Fix hygiene

b48c8d9

Fix untitled to work for old provider too

23329a6

DonJayamanne requested changes Jun 8, 2020

View reviewed changes

rchiodo commented Jun 8, 2020

View reviewed changes

Some feedback and attempt to fix URI problem on linux/mac

496b8be

rchiodo requested a review from DonJayamanne June 8, 2020 20:22

IanMatthewHuff reviewed Jun 8, 2020

View reviewed changes

src/client/common/application/customEditorService.ts Outdated Show resolved Hide resolved

IanMatthewHuff reviewed Jun 8, 2020

View reviewed changes

src/client/common/experiments/groups.ts Outdated Show resolved Hide resolved

Review feedback

615caf5

IanMatthewHuff approved these changes Jun 8, 2020

View reviewed changes

DonJayamanne approved these changes Jun 8, 2020

View reviewed changes

rchiodo merged commit dc32568 into master Jun 8, 2020

rchiodo deleted the rchiodo/renable_custom_editor branch June 8, 2020 23:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the changes necessary to use the new custom editor #12188

Implement the changes necessary to use the new custom editor #12188

rchiodo commented Jun 8, 2020

rchiodo commented Jun 8, 2020

rchiodo commented Jun 8, 2020

DonJayamanne Jun 8, 2020

rchiodo Jun 8, 2020

DonJayamanne Jun 8, 2020

rchiodo Jun 8, 2020

DonJayamanne Jun 8, 2020

rchiodo Jun 8, 2020

rchiodo Jun 8, 2020

rchiodo Jun 8, 2020

DonJayamanne Jun 8, 2020

rchiodo Jun 8, 2020

DonJayamanne Jun 8, 2020

rchiodo Jun 8, 2020

DonJayamanne Jun 8, 2020

rchiodo Jun 8, 2020

rchiodo Jun 8, 2020

sonarqubecloud bot commented Jun 8, 2020

Implement the changes necessary to use the new custom editor #12188

Implement the changes necessary to use the new custom editor #12188

Conversation

rchiodo commented Jun 8, 2020

rchiodo commented Jun 8, 2020

rchiodo commented Jun 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 8, 2020